Skip to content

Comments

feat: add skill activation system and switch to OpenRouter model#4

Merged
fdematos merged 3 commits intomainfrom
feat/skill-activation-and-openrouter
Jan 14, 2026
Merged

feat: add skill activation system and switch to OpenRouter model#4
fdematos merged 3 commits intomainfrom
feat/skill-activation-and-openrouter

Conversation

@fdematos
Copy link
Contributor

Summary

  • Add use_when field for skills to trigger automatic activation
  • Create skill tool for loading skills dynamically
  • Inject <skill-activation-rules> block into system prompt
  • Switch GitHub Actions model to openrouter/mistralai/devstral-2512:free

Changes

File Description
src/tools/skill.ts New tool for loading skills
src/index.ts Skill activation block injection
src/loaders.ts Add use_when field support
skill/code-simplify/SKILL.md Add activation trigger
.github/workflows/*.yml Switch to OpenRouter model

- Add use_when field for skills to trigger automatic activation
- Create skill tool for loading skills dynamically
- Inject skill-activation-rules block into system prompt
- Switch GitHub Actions model to openrouter/mistralai/devstral-2512:free
@opencode-agent
Copy link
Contributor

ProviderModelNotFoundError

opencode session  |  github run

@opencode-agent
Copy link
Contributor

Now let me analyze the code quality, potential bugs, and suggest improvements for this pull request:

Code Quality Review

1. src/tools/skill.ts

Code Quality Issues:

  • Line 86: The useWhen!.replace(/"/g, "&quot;").replace(/\n/g, " ").trim() uses non-null assertion (!) which could cause runtime errors if useWhen is undefined. This should be handled more safely.
  • Line 177-179: Error message construction could be expensive for large skill lists. Consider limiting the number of skills shown in error messages.
  • Line 182-191: The skill content loading and formatting could benefit from better error handling around file operations.

Potential Bugs:

  • Line 131-137: The priority order comment says "lowest to highest" but the actual implementation puts plugin skills first, which might not be the intended priority.
  • Line 140-142: The skill deduplication by name could cause issues if different scopes have skills with the same name but different content.

Suggestions:

  • Add input validation for the name parameter in the skill tool execution
  • Consider adding caching with TTL instead of indefinite caching
  • Add logging for skill discovery and loading operations

2. src/index.ts

Code Quality Issues:

  • Line 86: The useWhen! non-null assertion is unsafe. Should check if s.useWhen exists first.
  • Line 86: The regex replacement could be more robust to handle edge cases in trigger text.
  • Line 112-115: The skill activation block logic could be extracted to a separate function for better testability.

Potential Bugs:

  • Line 112-115: If skillsWithTriggers is empty, skillActivationBlock becomes null, but there's no null check when it's used in line 422-424.
  • Line 422-424: The experimental system transform could fail if skillActivationBlock is null but the check might not catch all cases.

Suggestions:

  • Add more comprehensive logging for skill activation system
  • Consider adding metrics for skill usage and activation
  • Add validation for the XML format generation

3. src/loaders.ts

Code Quality Issues:

  • Line 181: The useWhen: data.use_when mapping could be more explicit about the field name transformation.
  • Line 24-24: The use_when field in the interface should have better documentation about its purpose and format.

Suggestions:

  • Add validation for the use_when field format
  • Consider adding a helper function to normalize field names consistently

4. skill/code-simplify/SKILL.md

Code Quality Issues:

  • The use_when trigger text is quite long and specific. Consider if this level of specificity is maintainable.
  • The trigger mentions "REQUIRED" which might be confusing in the context of skill activation.

Suggestions:

  • Make the trigger text more concise and maintainable
  • Consider adding examples of when this skill should be activated

5. GitHub Workflows

Code Quality Issues:

  • Both workflows now use the same model (openrouter/mistralai/devstral-2512:free). This is good for consistency but consider if different workflows might need different models.
  • The model specification is quite long and specific. Consider if this could be parameterized.

Suggestions:

  • Add comments explaining why this specific model was chosen
  • Consider adding model performance monitoring
  • Add error handling for model availability issues

Overall Assessment

The pull request introduces a well-structured skill activation system with these key components:

  1. Skill Tool: A new tool for loading and executing skills dynamically
  2. Skill Activation: Automatic triggering based on use_when conditions
  3. System Prompt Integration: Injection of skill activation rules into the system prompt
  4. Model Switch: Migration to OpenRouter model

Strengths:

  • Clean separation of concerns between skill discovery, loading, and execution
  • Good use of TypeScript interfaces and types
  • Comprehensive error handling in most places
  • Clear logging structure

Areas for Improvement:

  1. Safety: Replace non-null assertions with proper null checks
  2. Error Handling: More robust error handling around file operations and skill loading
  3. Testing: The new functionality should have comprehensive tests
  4. Documentation: Better documentation for the use_when field format and expected behavior
  5. Performance: Consider caching strategies and performance implications of skill discovery

Potential Bugs to Address:

  1. The non-null assertion on useWhen could cause runtime errors
  2. Skill deduplication logic might have unintended consequences
  3. XML generation for skill activation rules needs validation
  4. Error messages could be too verbose with large skill lists

The changes are generally well-implemented but would benefit from additional safety checks and more comprehensive testing.

New%20session%20-%202026-01-14T18%3A31%3A40.061Z
opencode session  |  github run

- Fix unsafe non-null assertion on useWhen field
- Clarify skill merge priority comments (project > global > plugin)
- Extract buildSkillActivationBlock to separate module for testability
- Add escapeTrigger helper function
- Add 20 new tests for skill system (147 total)
- Strengthen code-simplify skill activation trigger
@opencode-agent
Copy link
Contributor

Let me analyze the code quality, potential bugs, and suggest improvements for this pull request:

Code Quality Review

1. src/tools/skill.ts

Code Quality Issues:

  • Line 86: The useWhen!.replace(/"/g, "&quot;").replace(/\n/g, " ").trim() uses non-null assertion (!) which could cause runtime errors if useWhen is undefined. This should be handled more safely.
  • Line 177-179: Error message construction could be expensive for large skill lists. Consider limiting the number of skills shown in error messages.
  • Line 182-191: The skill content loading and formatting could benefit from better error handling around file operations.

Potential Bugs:

  • Line 131-137: The priority order comment says "lowest to highest" but the actual implementation puts plugin skills first, which might not be the intended priority.
  • Line 140-142: The skill deduplication by name could cause issues if different scopes have skills with the same name but different content.

Suggestions:

  • Add input validation for the name parameter in the skill tool execution
  • Consider adding caching with TTL instead of indefinite caching
  • Add logging for skill discovery and loading operations

2. src/index.ts

Code Quality Issues:

  • Line 86: The useWhen! non-null assertion is unsafe. Should check if s.useWhen exists first.
  • Line 86: The regex replacement could be more robust to handle edge cases in trigger text.
  • Line 112-115: The skill activation block logic could be extracted to a separate function for better testability.

Potential Bugs:

  • Line 112-115: If skillsWithTriggers is empty, skillActivationBlock becomes null, but there's no null check when it's used in line 422-424.
  • Line 422-424: The experimental system transform could fail if skillActivationBlock is null but the check might not catch all cases.

Suggestions:

  • Add more comprehensive logging for skill activation system
  • Consider adding metrics for skill usage and activation
  • Add validation for the XML format generation

3. src/loaders.ts

Code Quality Issues:

  • Line 181: The useWhen: data.use_when mapping could be more explicit about the field name transformation.
  • Line 24-24: The use_when field in the interface should have better documentation about its purpose and format.

Suggestions:

  • Add validation for the use_when field format
  • Consider adding a helper function to normalize field names consistently

4. skill/code-simplify/SKILL.md

Code Quality Issues:

  • The use_when trigger text is quite long and specific. Consider if this level of specificity is maintainable.
  • The trigger mentions "REQUIRED" which might be confusing in the context of skill activation.

Suggestions:

  • Make the trigger text more concise and maintainable
  • Consider adding examples of when this skill should be activated

5. GitHub Workflows

Code Quality Issues:

  • Both workflows now use the same model (openrouter/mistralai/devstral-2512:free). This is good for consistency but consider if different workflows might need different models.
  • The model specification is quite long and specific. Consider if this could be parameterized.

Suggestions:

  • Add comments explaining why this specific model was chosen
  • Consider adding model performance monitoring
  • Add error handling for model availability issues

Overall Assessment

The pull request introduces a well-structured skill activation system with these key components:

  1. Skill Tool: A new tool for loading and executing skills dynamically
  2. Skill Activation: Automatic triggering based on use_when conditions
  3. System Prompt Integration: Injection of skill activation rules into the system prompt
  4. Model Switch: Migration to OpenRouter model

Strengths:

  • Clean separation of concerns between skill discovery, loading, and execution
  • Good use of TypeScript interfaces and types
  • Comprehensive error handling in most places
  • Clear logging structure

Areas for Improvement:

  1. Safety: Replace non-null assertions with proper null checks
  2. Error Handling: More robust error handling around file operations and skill loading
  3. Testing: The new functionality should have comprehensive tests
  4. Documentation: Better documentation for the use_when field format and expected behavior
  5. Performance: Consider caching strategies and performance implications of skill discovery

Potential Bugs to Address:

  1. The non-null assertion on useWhen could cause runtime errors
  2. Skill deduplication logic might have unintended consequences
  3. XML generation for skill activation rules needs validation
  4. Error messages could be too verbose with large skill lists

The changes are generally well-implemented but would benefit from additional safety checks and more comprehensive testing.

New%20session%20-%202026-01-14T19%3A00%3A38.835Z
opencode session  |  github run

@fdematos fdematos merged commit d237475 into main Jan 14, 2026
2 checks passed
@fdematos fdematos deleted the feat/skill-activation-and-openrouter branch January 14, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant